New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add autofix to media-feature-parentheses-space-inside #3720
Add autofix to media-feature-parentheses-space-inside #3720
Conversation
I had to fix snapshots. Now the order of the errors is more correct. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@yepninja This is looking fab! Thanks a lot. It's great to see the value parser being put to use.
I've made one very minor request to do with the name of a variable, otherwise this LGTM.
@@ -31,50 +31,62 @@ const rule = function(expectation) { | |||
// will be at atRule.raws.params.raw | |||
const params = _.get(atRule, "raws.params.raw", atRule.params); | |||
const indexBoost = atRuleParamIndex(atRule); | |||
const errors = []; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we rename this to violations
, please?
It's a convention we've adopted to:
- help distinguish between things caught by the linter and actual errors, for example those thrown by the parsers.
- act as an intermediary before those things are categorised into a particular severity e.g. "error" or "warning".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed. No problem at all.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done!
media-feature-parentheses-space-inside
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@yepninja I have added small suggestion.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@yepninja Thanks for making the change. LGTM.
|
#3585